From 0d5d5eca4afa35332891112f5f25c2f2cba790a2 Mon Sep 17 00:00:00 2001 From: Wei Liu Date: Thu, 4 Sep 2014 23:43:13 +0100 Subject: [PATCH] libxl: disallow attaching the same device more than once Originally the code allowed users to attach the same device more than once. It just stupidly overwrites xenstore entries. This is bogus as frontend will be very confused. Introduce a helper function to check if the device to be written to xenstore already exists. A new error code is also introduced. The check and add are within one xs transaction. Signed-off-by: Wei Liu Acked-by: Ian Campbell --- tools/libxl/libxl.c | 70 +++++++++++++++++++++++++++++++----- tools/libxl/libxl_device.c | 19 ++++++++++ tools/libxl/libxl_internal.h | 3 ++ tools/libxl/libxl_pci.c | 35 +++++++++++++----- tools/libxl/libxl_types.idl | 1 + 5 files changed, 112 insertions(+), 16 deletions(-) diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c index 9bb1a90bbe..ad3495a804 100644 --- a/tools/libxl/libxl.c +++ b/tools/libxl/libxl.c @@ -1893,6 +1893,7 @@ void libxl__device_vtpm_add(libxl__egc *egc, uint32_t domid, flexarray_t *back; libxl__device *device; int rc; + xs_transaction_t t = XBT_NULL; rc = libxl__device_vtpm_setdefault(gc, vtpm); if (rc) goto out; @@ -1932,10 +1933,30 @@ void libxl__device_vtpm_add(libxl__egc *egc, uint32_t domid, flexarray_append(front, "handle"); flexarray_append(front, GCSPRINTF("%d", vtpm->devid)); - libxl__device_generic_add(gc, XBT_NULL, device, - libxl__xs_kvs_of_flexarray(gc, back, back->count), - libxl__xs_kvs_of_flexarray(gc, front, front->count), - NULL); + for (;;) { + rc = libxl__xs_transaction_start(gc, &t); + if (rc) goto out; + + rc = libxl__device_exists(gc, t, device); + if (rc < 0) goto out; + if (rc == 1) { /* already exists in xenstore */ + LOG(ERROR, "device already exists in xenstore"); + aodev->action = LIBXL__DEVICE_ACTION_ADD; /* for error message */ + rc = ERROR_DEVICE_EXISTS; + goto out; + } + + libxl__device_generic_add(gc, t, device, + libxl__xs_kvs_of_flexarray(gc, back, + back->count), + libxl__xs_kvs_of_flexarray(gc, front, + front->count), + NULL); + + rc = libxl__xs_transaction_commit(gc, &t); + if (!rc) break; + if (rc < 0) goto out; + } aodev->dev = device; aodev->action = LIBXL__DEVICE_ACTION_ADD; @@ -1943,6 +1964,7 @@ void libxl__device_vtpm_add(libxl__egc *egc, uint32_t domid, rc = 0; out: + libxl__xs_transaction_abort(gc, &t); aodev->rc = rc; if(rc) aodev->callback(egc, aodev); return; @@ -2209,6 +2231,15 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid, goto out; } + rc = libxl__device_exists(gc, t, device); + if (rc < 0) goto out; + if (rc == 1) { /* already exists in xenstore */ + LOG(ERROR, "device already exists in xenstore"); + aodev->action = LIBXL__DEVICE_ACTION_ADD; /* for error message */ + rc = ERROR_DEVICE_EXISTS; + goto out; + } + switch (disk->backend) { case LIBXL_DISK_BACKEND_PHY: dev = disk->pdev_path; @@ -2998,6 +3029,7 @@ void libxl__device_nic_add(libxl__egc *egc, uint32_t domid, flexarray_t *back; libxl__device *device; int rc; + xs_transaction_t t = XBT_NULL; rc = libxl__device_nic_setdefault(gc, nic, domid); if (rc) goto out; @@ -3068,10 +3100,31 @@ void libxl__device_nic_add(libxl__egc *egc, uint32_t domid, flexarray_append(front, "mac"); flexarray_append(front, libxl__sprintf(gc, LIBXL_MAC_FMT, LIBXL_MAC_BYTES(nic->mac))); - libxl__device_generic_add(gc, XBT_NULL, device, - libxl__xs_kvs_of_flexarray(gc, back, back->count), - libxl__xs_kvs_of_flexarray(gc, front, front->count), - NULL); + + for (;;) { + rc = libxl__xs_transaction_start(gc, &t); + if (rc) goto out; + + rc = libxl__device_exists(gc, t, device); + if (rc < 0) goto out; + if (rc == 1) { /* already exists in xenstore */ + LOG(ERROR, "device already exists in xenstore"); + aodev->action = LIBXL__DEVICE_ACTION_ADD; /* for error message */ + rc = ERROR_DEVICE_EXISTS; + goto out; + } + + libxl__device_generic_add(gc, t, device, + libxl__xs_kvs_of_flexarray(gc, back, + back->count), + libxl__xs_kvs_of_flexarray(gc, front, + front->count), + NULL); + + rc = libxl__xs_transaction_commit(gc, &t); + if (!rc) break; + if (rc < 0) goto out; + } aodev->dev = device; aodev->action = LIBXL__DEVICE_ACTION_ADD; @@ -3079,6 +3132,7 @@ void libxl__device_nic_add(libxl__egc *egc, uint32_t domid, rc = 0; out: + libxl__xs_transaction_abort(gc, &t); aodev->rc = rc; if (rc) aodev->callback(egc, aodev); return; diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c index f8a2e1ba92..045212fa5b 100644 --- a/tools/libxl/libxl_device.c +++ b/tools/libxl/libxl_device.c @@ -40,6 +40,25 @@ char *libxl__device_backend_path(libxl__gc *gc, libxl__device *device) device->domid, device->devid); } +/* Returns 1 if device exists, 0 if not, ERROR_* (<0) on error. */ +int libxl__device_exists(libxl__gc *gc, xs_transaction_t t, + libxl__device *device) +{ + int rc; + char *be_path = libxl__device_backend_path(gc, device); + const char *dir; + + be_path = libxl__device_backend_path(gc, device); + rc = libxl__xs_read_checked(gc, t, be_path, &dir); + + if (rc) + return rc; + + if (dir) + return 1; + return 0; +} + int libxl__parse_backend_path(libxl__gc *gc, const char *path, libxl__device *dev) diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h index 3b8f74edf6..fafef5a420 100644 --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -1033,6 +1033,9 @@ _hidden int libxl__device_console_add(libxl__gc *gc, uint32_t domid, libxl__device_console *console, libxl__domain_build_state *state); +/* Returns 1 if device exists, 0 if not, ERROR_* (<0) on error. */ +_hidden int libxl__device_exists(libxl__gc *gc, xs_transaction_t t, + libxl__device *device); _hidden int libxl__device_generic_add(libxl__gc *gc, xs_transaction_t t, libxl__device *device, char **bents, char **fents, char **ro_fents); _hidden char *libxl__device_backend_path(libxl__gc *gc, libxl__device *device); diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c index 0500cf3593..38a9642e61 100644 --- a/tools/libxl/libxl_pci.c +++ b/tools/libxl/libxl_pci.c @@ -124,6 +124,8 @@ static int libxl__device_pci_add_xenstore(libxl__gc *gc, uint32_t domid, libxl_d char *num_devs, *be_path; int num = 0; xs_transaction_t t; + libxl__device *device; + int rc; be_path = libxl__sprintf(gc, "%s/backend/pci/%d/0", libxl__xs_get_dompath(gc, 0), domid); num_devs = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/num_devs", be_path)); @@ -148,15 +150,32 @@ static int libxl__device_pci_add_xenstore(libxl__gc *gc, uint32_t domid, libxl_d if (!starting) flexarray_append_pair(back, "state", libxl__sprintf(gc, "%d", 7)); -retry_transaction: - t = xs_transaction_start(ctx->xsh); - libxl__xs_writev(gc, t, be_path, - libxl__xs_kvs_of_flexarray(gc, back, back->count)); - if (!xs_transaction_end(ctx->xsh, t, 0)) - if (errno == EAGAIN) - goto retry_transaction; + GCNEW(device); + libxl__device_from_pcidev(gc, domid, pcidev, device); - return 0; + for (;;) { + rc = libxl__xs_transaction_start(gc, &t); + if (rc) goto out; + + rc = libxl__device_exists(gc, t, device); + if (rc < 0) goto out; + if (rc == 1) { + LOG(ERROR, "device already exists in xenstore"); + rc = ERROR_DEVICE_EXISTS; + goto out; + } + + libxl__xs_writev(gc, t, be_path, + libxl__xs_kvs_of_flexarray(gc, back, back->count)); + + rc = libxl__xs_transaction_commit(gc, &t); + if (!rc) break; + if (rc < 0) goto out; + } + +out: + libxl__xs_transaction_abort(gc, &t); + return rc; } static int libxl__device_pci_remove_xenstore(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev) diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl index 1cd635ea8c..f1fcbc312b 100644 --- a/tools/libxl/libxl_types.idl +++ b/tools/libxl/libxl_types.idl @@ -60,6 +60,7 @@ libxl_error = Enumeration("error", [ (-14, "UNKNOWN_CHILD"), (-15, "LOCK_FAIL"), (-16, "JSON_CONFIG_EMPTY"), + (-17, "DEVICE_EXISTS"), ], value_namespace = "") libxl_domain_type = Enumeration("domain_type", [ -- 2.30.2